hello @arifbalik

Thanks for your contribution! Please find below few comments:

• Could you detail the naming delta?
• Would it be possible to enhance https://github.com/zephyrproject-rtos/hal_stm32/blob/main/scripts/genpinctrl/stm32-pinctrl-config.yaml instead of creating a new file just for the Ethernet? Or do you observe some other differences that require a separate file?
• It also seems that you could close the dts: update stm32mp1 series ethernet pins #288 and add directly the commit in this one as it addresses the same topic
• last request: could you refactor your commit message to 75 char per line?

I was trying to bring up ethernet on stm32mp1 and realized the pin data is quite different than what was specified in the default yml file, I'll see if I can put it inside the default one but it seems unlikely.

Also last time I updated the pinctrl files @erwango suggested that I should open another PR for the dtsi files since they were quite large, i can put it here as well

I updated the pinctrl files @erwango suggested that I should open another PR for the dtsi

Maybe I wasn't clear enough, I wanted to mean splitting into disctinct commits, not PRs.

oh my bad

I was trying to bring up ethernet on stm32mp1 and realized the pin data is quite different than what was specified in the default yml file, I'll see if I can put it inside the default one but it seems unlikely.

perhaps you could just add

- name: ETH_MP1x
  match: "^ETH[12]_(CLK|COL|CRS|CRS_DV|GTX_CLK|MDC|MDIO|PHY_INTN|PPS_OUT|REF_CLK|RX_CLK|RX_CTL|RX_DV|RX_ER|RXD[0-3]|TX_CLK|TX_CTL|TX_EN|TX_ER|TXD[0-3])$"
  slew-rate: very-high-speed

I was trying to bring up ethernet on stm32mp1 and realized the pin data is quite different than what was specified in the default yml file, I'll see if I can put it inside the default one but it seems unlikely.

perhaps you could just add

- name: ETH_MP1x
  match: "^ETH[12]_(CLK|COL|CRS|CRS_DV|GTX_CLK|MDC|MDIO|PHY_INTN|PPS_OUT|REF_CLK|RX_CLK|RX_CTL|RX_DV|RX_ER|RXD[0-3]|TX_CLK|TX_CTL|TX_EN|TX_ER|TXD[0-3])$"
  slew-rate: very-high-speed

The problem is that some of the definitions in the pin data matches some regex expressions inside stm32-pinctrl-config.yaml but not all, so if we just add the expression you provided, we would get some definitions twice.

For example some definitions will match the following regex and this is from stm32-pinctrl-config.yaml

- name: ETH (Extended)
  match: "^ETH\\d+_(?:MDC|MDIO|PHY_INTN|PPS_OUT|CLK|TX_ER)$"
  slew-rate: very-high-speed

But none would match against this;

- name: ETH
  match: "^ETH+_(?:COL$|CRS|CRS_DV|MDC|MDIO|PPS_OUT|REF_CLK|RX_CLK|RX_DV|RX_ER||RXD[0-3]|TX_CLK|TX_EN|TXD[0-3])$"
  slew-rate: very-high-speed

Maybe I could remove the expressions inside EH_MP1x that causes the double match, but then the pinctrl file would call some ETH pins as "extended" even though they are not.

I was trying to bring up ethernet on stm32mp1 and realized the pin data is quite different than what was specified in the default yml file, I'll see if I can put it inside the default one but it seems unlikely.

perhaps you could just add

- name: ETH_MP1x
  match: "^ETH[12]_(CLK|COL|CRS|CRS_DV|GTX_CLK|MDC|MDIO|PHY_INTN|PPS_OUT|REF_CLK|RX_CLK|RX_CTL|RX_DV|RX_ER|RXD[0-3]|TX_CLK|TX_CTL|TX_EN|TX_ER|TXD[0-3])$"
  slew-rate: very-high-speed

The problem is that some of the definitions in the pin data matches some regex expressions inside stm32-pinctrl-config.yaml but not all, so if we just add the expression you provided, we would get some definitions twice.

For example some definitions will match the following regex and this is from stm32-pinctrl-config.yaml

- name: ETH (Extended)
  match: "^ETH\\d+_(?:MDC|MDIO|PHY_INTN|PPS_OUT|CLK|TX_ER)$"
  slew-rate: very-high-speed

But none would match against this;

- name: ETH
  match: "^ETH+_(?:COL$|CRS|CRS_DV|MDC|MDIO|PPS_OUT|REF_CLK|RX_CLK|RX_DV|RX_ER||RXD[0-3]|TX_CLK|TX_EN|TXD[0-3])$"
  slew-rate: very-high-speed

Maybe I could remove the expressions inside EH_MP1x that causes the double match, but then the pinctrl file would call some ETH pins as "extended" even though they are not.

Is Completing the ETH (Extended) not sufficient?
Could you provide some examples of names that would not match, explaining why? This would help us better understand the issue.

If we are unable to enhance the stm32-pinctrl-config.yaml, an alternative could be to include the changes in stm32mp1-pinctrl-config.yaml to only override the ETH section.

@arifbalik

Any update planned?
FYI I just pushed in draft the support of the eth1 base you this PR: zephyrproject-rtos/zephyr#96134

@arifbalik

Any update planned? FYI I just pushed in draft the support of the eth1 base you this PR: zephyrproject-rtos/zephyr#96134

@arnopo I've returned to your email but i guess there was a problem, I was also working with ethernet and had a very similar branch to yours. But it was not ready for upstream.

I havent been able to work on this for a while. But i can get back to it.

Let me see if i can change the pin config like you suggested. I can get back to you by the end of the week.

@arifbalik
Any update planned? FYI I just pushed in draft the support of the eth1 base you this PR: zephyrproject-rtos/zephyr#96134

@arnopo I've returned to your email but i guess there was a problem, I was also working with ethernet and had a very similar branch to yours. But it was not ready for upstream.

I did not received answer from you, but I saw that you push some related code on your GitHub. Testing it was not functional.
But I reused a part of your device tree. So don't hesitate to claim that I add your sign-off on some part of the code I pushed.

I havent been able to work on this for a while. But i can get back to it.

Let me see if i can change the pin config like you suggested. I can get back to you by the end of the week.
Thank!

@arnopo I did managed to put the regex inside the original yaml file. However there is a logical seperation now, because some of the pins match the regex ETH (Extended), and I put the rest in the ETH_STM32MP1, so the pins are seperated by ETH (Extended) and ETH_STM32MP1 which may cause confusion.

@arnopo Also it seems like mp15x series have different namings than mp13x series, so i left it out when regenerating the pins

So don't hesitate to claim that I add your sign-off on some part of the code I pushed

That would be great i you could do it

@arnopo Also it seems like mp15x series have different namings than mp13x series, so i left it out when regenerating the pins

The ethernet is not supported on stm32mp15 Cortex-M4 (The ethernet is managed by the Linux on Cortex- A ) so that seems ok to me.
@erwango: any opinion on that?

@arnopo I did managed to put the regex inside the original yaml file. However there is a logical seperation now, because some of the pins match the regex ETH (Extended), and I put the rest in the ETH_STM32MP1, so the pins are seperated by ETH (Extended) and ETH_STM32MP1 which may cause confusion.

LGTM but need @erwango feedback

So don't hesitate to claim that I add your sign-off on some part of the code I pushed

That would be great i you could do it

I will add your sign-off in commit that impact the DTS and the eth_stm32_hal drivers. in my update of the PR

@arnopo Also it seems like mp15x series have different namings than mp13x series, so i left it out when regenerating the pins

That is not ideal. Script is made to be used by anyone to add new configuration, there should not be leftover files to deal with.
Either eth pins for mp15 files should not be generated, but we miss configurability in the script for this, either they are generated anyway and we live with them.

@arifbalik Following discussion with @arnopo It is ok to merge mp15 eth pin configurations. The only change would be to change from ETH_STM32MP13 to ETH_STM32MP1.
Can you do this change ?